Adding dra-kubelet communication volumes#1297
Conversation
✅ Deploy Preview for kubernetes-sigs-kmm ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: yevgeny-shnaidman The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
kubernetes-prow
Bot
added
approved
|
/assing @ybettan |
|
/assign @ybettan |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1297 +/- ##
==========================================
- Coverage 79.09% 73.90% -5.19%
==========================================
Files 51 67 +16
Lines 5109 5101 -8
==========================================
- Hits 4041 3770 -271
- Misses 882 1158 +276
+ Partials 186 173 -13 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
/test pull-kernel-module-management-lint |
| }, | ||
| { | ||
| Name: "HEALTHCHECK_PORT", | ||
| Value: fmt.Sprintf("%d", draHealthcheckPort), |
There was a problem hiding this comment.
What happens if 2 DRA pods lands on the same node and both bind the 51515 port? One of them will probably fail isn't it?
There was a problem hiding this comment.
dra pods are deployed by daemonset, so 2 pods cannot land on the same node
| containerVolumeMounts := []v1.VolumeMount{ | ||
| {Name: kubeletPluginsVolumeName, MountPath: kubeletPluginsPath}, | ||
| {Name: kubeletPluginsRegistryVolumeName, MountPath: kubeletPluginsRegistryPath}, | ||
| {Name: cdiVolumeName, MountPath: cdiPath}, |
There was a problem hiding this comment.
Here the CDI volume mount override the user mounts unlike the ENV variables that allows the user to override the pre-set variables. Is this by design? Shouldn't we have a consistent behavior?
There was a problem hiding this comment.
they don't override user mount. See line 503
There was a problem hiding this comment.
changed the order, so that user's volume's definition will take precedence
| Spec: v1.PodSpec{ | ||
| InitContainers: generatePodContainerSpec(mod.Spec.DRA.InitContainer, "dra-init", nil), | ||
| Containers: generatePodContainerSpec(&mod.Spec.DRA.Container, "dra", containerVolumeMounts), | ||
| Containers: []v1.Container{draContainer}, |
There was a problem hiding this comment.
Why don't we keep using generatePodContainerSpec and append additional fields? This mean that what ever is added to the output of generatePodContainerSpec will not be inherited by this container.
yevgeny-shnaidman
force-pushed
the
yevgeny/add-dra-volumes
branch
from
328756e to
96d9922
Compare
This commit add mapping of a number of volumes and envs that are necessary for DRA driver <--> kubelet communication
yevgeny-shnaidman
force-pushed
the
yevgeny/add-dra-volumes
branch
from
96d9922 to
7d47413
Compare
|
/lgtm |
kubernetes-prow
Bot
added
the
lgtm
|
/test pull-kernel-module-management-lint |
3 participants
This commit add mapping of a number of volumes and envs that are necessary for DRA driver <--> kubelet communication